fix: restore currency_id int->str coercion lost in model regen by RadCod3 · Pull Request #83 · RadCod3/LamPyrid

RadCod3 · 2026-06-03T20:28:46Z

Problem

Budget endpoints (e.g. list_budget_limits / get_budget_spending) fail with a Pydantic validation error:

1 validation error for BudgetLimitArray
data.0.attributes.pc_spent.0.currency_id
  Input should be a valid string [type=string_type, input_value=25, input_type=int]

The Firefly III API (Issue #43) returns currency_id as an int inside spent / pc_spent (ArrayEntryWithCurrencyAndSum) arrays, even though the OpenAPI spec types it as a string.

Root cause

A BeforeValidator coercion (int → str) was originally added in 346ce63 ("fix: add temp fix for availble_budgets"), but it lived directly in the auto-generated firefly_models.py and was silently wiped out when the models were regenerated in 55cdb0b ("Regen firefly models with latest openapi spec").

Fix

Restore the BeforeValidator(_coerce_to_str) on ArrayEntryWithCurrencyAndSum.currency_id.
Add an idempotent apply_manual_fixes() step to scripts/update_schema.py, invoked right after datamodel-codegen runs, so the patch is re-injected automatically after every regeneration — preventing this fix from being lost again.

Testing

Verified the model now coerces currency_id: 25 (int) → '25' (str), including via the full BudgetLimitArray payload shape that was failing.
uv run pytest tests/unit/test_models.py — 17 passed.
ruff format / ruff check clean.

🤖 Generated with Claude Code

The Firefly III API (Issue #43) returns currency_id as an int in spent/pc_spent (ArrayEntryWithCurrencyAndSum) arrays, while the OpenAPI spec types it as a string. A BeforeValidator coercion was added in 346ce63 but wiped out when models were regenerated in 55cdb0b, causing budget endpoints (e.g. list_budget_limits) to fail with: data.0.attributes.pc_spent.0.currency_id Input should be a valid string [type=string_type, input_value=25] This restores the BeforeValidator on ArrayEntryWithCurrencyAndSum and adds an apply_manual_fixes() step to scripts/update_schema.py so the patch is re-injected idempotently after every regeneration, preventing the fix from being lost again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai · 2026-06-03T20:28:56Z

Warning

Review limit reached

@RadCod3, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 51 minutes and 49 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef036368-8288-4867-b61b-c8c2f3bab7bf

📥 Commits

Reviewing files that changed from the base of the PR and between 1131559 and f38e551.

📒 Files selected for processing (2)

src/lampyrid/scripts/update_schema.py
tests/unit/test_scripts.py

📝 Walkthrough

Walkthrough

This PR adds a Pydantic validator workaround for a Firefly III API schema defect where currency_id is returned as int instead of str. The validator coerces the input during model validation, and automation in the schema update script persists this fix across future schema regenerations.

Changes

Firefly III currency_id validator workaround

Layer / File(s)	Summary
Pydantic validator for currency_id coercion `src/lampyrid/models/firefly_models.py`	Imports `Annotated` and `BeforeValidator` from Pydantic, defines `_coerce_to_str` helper to safely convert `int`/`None` to `str`/`None`, and rewrites `ArrayEntryWithCurrencyAndSum.currency_id` field to apply the validator during model instantiation.
Schema regeneration automation `src/lampyrid/scripts/update_schema.py`	Adds `apply_manual_fixes()` function that post-processes the generated `firefly_models.py` file to idempotently inject Pydantic validator imports and rewrite the `currency_id` field definition. Integrates into `main()` workflow to ensure the manual fix persists after schema regeneration and before code formatting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

RadCod3/LamPyrid#63: Addresses the same Firefly III currency_id validation bug affecting test_get_available_budget by introducing BeforeValidator coercion.

Poem

🐰 A currency that hops between types,
Now caught mid-leap by the validator's swipes—
Int to string, the fix is complete,
Automation ensures it's preserved so neat!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: restoring lost currency_id type coercion after model regeneration.
Description check	✅ Passed	The description thoroughly explains the problem, root cause, and solution, with testing details and commit references that directly relate to the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/currency-id-int-coercion

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov-commenter · 2026-06-03T20:33:39Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.59%. Comparing base (297aa57) to head (f38e551).

Files with missing lines	Patch %	Lines
src/lampyrid/models/firefly_models.py	57.14%	3 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #83      +/-   ##
==========================================
- Coverage   97.64%   97.59%   -0.05%     
==========================================
  Files          22       22              
  Lines        3475     3499      +24     
==========================================
+ Hits         3393     3415      +22     
- Misses         82       84       +2

Flag	Coverage Δ
integration	`97.59% <88.88%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lampyrid/scripts/update_schema.py`:
- Around line 206-250: The apply_manual_fixes function currently always returns
True even when its replace() calls made no changes; update apply_manual_fixes to
detect whether any replacement actually happened by comparing the original
content to the modified content (or checking for the presence of the inserted
marker '_coerce_to_str' and the modified ArrayEntryWithCurrencyAndSum
annotation) before writing with models_path.write_text and returning True; if no
changes were applied, do not write and return False so callers (e.g., the code
path that expects the manual coercion for BudgetArray) can fail loudly.
- Around line 338-343: The manual Firefly III API workaround is only run in
main(), so regenerate_models() can leave firefly_models.py unpatched when called
directly; modify regenerate_models() to call apply_manual_fixes() after the
models file is written (the same place main() prints "Applying manual
fixes..."), using the same success/failure handling and informational output as
in main() so tests and callers that invoke regenerate_models() directly get the
patched file; reference the regenerate_models() function and the
apply_manual_fixes() call/site to locate the change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c6caf3c6-5afc-4266-b8dd-2404cdbfb984

📥 Commits

Reviewing files that changed from the base of the PR and between 297aa57 and 1131559.

📒 Files selected for processing (2)

src/lampyrid/models/firefly_models.py
src/lampyrid/scripts/update_schema.py

- apply_manual_fixes() now fails loudly: raises RuntimeError if any patch anchor is missing (datamodel-codegen output drift) and FileNotFoundError if the models file is absent, instead of silently reporting success and reintroducing the BudgetArray/pc_spent validation failure. - Move the apply_manual_fixes() call into regenerate_models() so every caller of that helper (e.g. tests, direct invocations) gets a patched models file, not just the CLI main() path. - Add unit tests covering patch application, idempotency, missing-anchor and missing-file failures, and regenerate_models integration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai Bot reviewed Jun 3, 2026

View reviewed changes

Comment thread src/lampyrid/scripts/update_schema.py Outdated

Comment thread src/lampyrid/scripts/update_schema.py Outdated

RadCod3 merged commit 8f1b53a into main Jun 3, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: restore currency_id int->str coercion lost in model regen#83

fix: restore currency_id int->str coercion lost in model regen#83
RadCod3 merged 2 commits into
mainfrom
fix/currency-id-int-coercion

RadCod3 commented Jun 3, 2026

Uh oh!

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

codecov-commenter commented Jun 3, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

RadCod3 commented Jun 3, 2026

Problem

Root cause

Fix

Testing

Uh oh!

coderabbitai Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

codecov-commenter commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading

codecov-commenter commented Jun 3, 2026 •

edited

Loading